Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[김예준] Sprint10 #261

Conversation

K777agent
Copy link
Collaborator

@K777agent K777agent commented Jul 19, 2024

요구사항

  • Github에 PR(Pull Request)을 만들어서 미션을 제출합니다.
  • 피그마 디자인에 맞게 페이지를 만들어 주세요.
  • 기존의 React, Typescript로 구현한 프로젝트와 별도로 진행합니다.
  • ��Next.js를 사용합니다

기본

상품 등록 페이지

  • 상품 등록 페이지 주소는 “/addboard” 입니다.
  • 게시판 이미지는 최대 한개 업로드가 가능합니다.
  • 각 input의 placeholder 값을 정확히 입력해주세요.
  • 이미지를 제외하고 input 에 모든 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.
  • 회원가입, 로그인 api를 사용하여 받은 accessToken을 사용하여 게시물 등록을 합니다.
  • ‘등록’ 버튼을 누르면 게시물 상세 페이지로 이동합니다.

상품 상세 페이지

  • 상품 상세 페이지 주소는 “/addboard/{id}” 입니다.
  • 댓글 input 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.
  • 활성화된 ‘등록' 버튼을 누르면 댓글이 등록됩니다

심화

주요 변경사항

  • 등록을 하는 부분에 대해서 어떻게 해야 될지 아직 미숙해서 제작을 못하였습니다.
  • 사이트의 주소를 id와 create로 제작하였습니다.

스크린샷

image
image
image

멘토에게

  • post를 올리는 방법에 대한 내용을 좀 더 자세히 설명해주시면 감사하겠습니다.
  • 전에 말씀 해주셨던 부분 중에서 recent, like 부분이 한글로 처리가 안되는 부분의 문제가 무엇인지 잘 모르겠습니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@K777agent K777agent added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jul 19, 2024
@K777agent K777agent requested a review from lhc0506 July 19, 2024 04:27
<div
key={key}
className={styles.dropDownItem}
onClick={() => handleOptionClick(key)}
>
{key}
{value}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 value로 바꿔줬기때문에 지금 dropdown에서 영어로 나오고 있어요!
만약 value로 여기를 바꾸면 내려오는 options도 다 바꿔줘야합니다!

@@ -9,14 +10,19 @@ import Button from '@/src/components/Button/Button'
import styles from './Article.module.scss'

export default function ArticleList() {
const [orderBy, setOrderBy] = useState('recent')
type OrderOption = 'recent' | 'like'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 dropdown으로 내려주는 친구들이있네요!
우선 보통 type은 컴포넌트 밖에서 선언을 해줘요.
그리고 oderOption에 해당하는 값들은 상수로 선언해주면 더 보기도 편하고 유지보수도 좋을 것 같아요!

const ODER_OPTION = {
  recent: '최신순',
  like: '인기순',
} as const

type OrderOption = keyof typeof ODER_OPTION

이렇게 key에 영어를 넣고 value에 한글을 넣으면 드롭다운에서 value로 보여줄 때 한글로 잘보일거에요!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아마 여기를 이렇게 바꾸면 타입에러가 조금씩 발생할 거에요. 그부분은 한번 고쳐서 타입세이프하게 만들어보세요 😃

Comment on lines +17 to +23
const url = useMemo(() => {
let baseUrl = `articles?page=1&pageSize=10&orderBy=${orderBy}`
if (searchTitle) {
baseUrl += `&keyword=${searchTitle}`
}
return baseUrl
}, [orderBy, searchTitle])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useMemo를 사용하여 url변화값을 잘 메모해주셨어요!

한가지 종종사용되는 부분을 추가적으로 말씀드리자면 orderByt나 keyword같이 검색에 필요한 파라미터들은 router를 이용하여 실제 url query에 넣기도합니다.
그렇게 했을 때 좋은 점은 실제로 유저의 행동을 history에 기억할 수 있다는 점이에요!
예를들어서 무언가를 검색하고 다른페이지로 넘어갔을 때, 뒤로가기를 누르면 그 검색한 키워드를 기억 할 수 있겠죠?!

return (
<>
<ArticleDetail />

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Comment on lines +5 to +7
<>
<CreateArticle />
</>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<>
<CreateArticle />
</>
<CreateArticle />

불필요한 fragment인것같아요!

혹시 CreateArticle컴포넌트를 따로 빼신 이유가 있으실까요??

Comment on lines +25 to 26
const fetechArticles = useFetechData<ArticleListResponse>(url)
const { data: ArticleList, isLoading } = fetechArticles
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const fetechArticles = useFetechData<ArticleListResponse>(url)
const { data: ArticleList, isLoading } = fetechArticles
const { data: articleList, isLoading } = useFetchData<ArticleListResponse>(url)

하나로 합칠 수 있을 것 같아요!
그리고 변수이름은 camelCase로 해주는 것이 일반적입니다~ 오타도 하나 있네요!

export default function CreateArticle() {
const url = `/articles`

const [formValues, setFormValues] = useState<FormValues>({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

creatArticle 안에 너무 많은 코드가 들어있어서 한눈에 들어오기가 어려운것같아요!
form관려한 부분을 hook으로 빼서 분리를 해주면 관심사가 분리되어 조금 더 매끄러운 코드가 될 것 같습니다!

width={282}
height={282}
/>
<button className={styles.xButton} onClick={handleImageDelete}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type이 명시가 안되어있으면 기본적으로 submit이 들어가요!
그래서 예상 외의 행동을 할 수도 있을 것 같습니다!

const { id } = router.query
const url = `/articles/${id}/comments`

const [comment, setComment] = useState<string>('')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 마찬가지로 comment에 관련한 로직을 useAddComment 정도로 따로 분리해주면 코드읽기에도 좋고 향후 재사용성도 늘어날것같아요!

Comment on lines +58 to +60
style={{
display: articleImage === '' ? 'none' : 'block',
}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 css파일을 따로 두고있으니 inline style보단 css파일에서 분리를 해준것이 더 통일성 있어보입니다!

Comment on lines +34 to +57
<div className={styles.container}>
<div className={styles.sectionTitle}>댓글 달기</div>
<form onSubmit={handleSubmit} className={styles.commentContainer}>
<textarea
className={styles.commentInput}
value={comment}
onChange={(e) => setComment(e.target.value)}
placeholder="댓글을 입력해주세요."
required
/>
{isLoading ? (
<Spinner />
) : (
<button
className={styles.submitButton}
type="submit"
disabled={isButtonDisabled}
>
등록
</button>
)}
{error && <div>{error}</div>}
</form>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

파일구조가 container안에 있는데, 여기서 컴포넌트까지 다 보여주는것이 조금 어색해보입니다!
컴포넌트를 따로 빼서 작성하는게 파일구조와 일관성이 있어보여요!

Comment on lines +52 to +79
<>
<div className={styles.contentSection}>
<div className={styles.content}>{comment.content}</div>
<Image
src={seemoreIcon}
width={24}
height={24}
alt="더보기 아이콘"
/>
</div>

<div className={styles.writerSection}>
<Image
src={defaultProfileIcon}
alt="기본 프로필 이미지"
width={32}
height={32}
/>
<div className={styles.writer}>
<div className={styles.nickname}>{comment.writer.nickname}</div>
<div className={styles.createdAt}>
{formatDate(comment.createdAt)}
</div>
</div>
</div>

<hr className={styles.hr} />
</>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이부분은 충분히 컴포넌트로 따로 빼서 사용이 가능할것같습니다!

@lhc0506 lhc0506 merged commit 415a5ef into codeit-bootcamp-frontend:Next-김예준 Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants